explicit diffusion part 2 - variable groups#974
Merged
travissluka merged 3 commits intodevelopfrom Nov 14, 2023
Merged
Conversation
kbhargava
approved these changes
Nov 14, 2023
Contributor
kbhargava
left a comment
kbhargava
left a comment
There was a problem hiding this comment.
Looks good. Builds and runs fine on orion. Will test a realistic case after the part3 PR. Thanks Travis!!
guillaumevernieres
approved these changes
Nov 14, 2023
Contributor
guillaumevernieres
left a comment
guillaumevernieres
left a comment
There was a problem hiding this comment.
Thanks @travissluka
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Previously the
EXPLICIT_DIFFUSIONblock could only use a single correlation length for all the variables.This PR adds "variable groups" so that multiple different lengths can be used for different variables.
Don't pay too much attention to the code changes, since some of that will change again once 3D support is added (it was mainly wrapping parts of the existing code in loops to handle groups). Instead, make sure the changes to the yaml input files make sense.
Changes to the yamls:
The calibration step now takes a list of groups that have their scales defined (note that this is just a group name, it is not actually associated with any model variables at this point). All the resulting parameters are saved to the same output file.
When using, for example in the dirac app,
groupsis required and does a mapping between the group name in the parameters input file (generated in the above step) and the actual variables in the model. If a model variable that the diffusion operator is supposed to process is not in any of the groups, a error will be raised.If you look at the test diracs, T (left) and S are the same as they were before, SSH dirac (right) has been added and uses a smaller length
